Skip to content

Fix bare scan/get converting hosted pins (#1437) - #1440

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-bare-mode-mixed-hosted-vendored
Oct 12, 2026
Merged

Fix bare scan/get converting hosted pins (#1437)#1440
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-bare-mode-mixed-hosted-vendored

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1437

Summary

A bare scan or get (no --mode) on a project that holds vendored packages and hosted pins now stops with exit 2 (mode_ambiguous). The error names the hosted-pinned packages and asks for --mode, and nothing is written. Before this change the run kept vendored mode and silently took every hosted pin over to vendored (exit 0, no warning). That undid a partial --mode hosted takeover (#1039) and per-package get --mode choices.

Root cause

mode_from_project_state (crates/socket-patch-cli/src/commands/mod.rs) picked the mode from the vendor ledger and the manifest only. Hosted mode keeps no ledger, so its pins never counted. The (vendored, !agent) arm returned Vendored even with hosted pins present, and the vendored flow then took over every pin it found.

Fix

Design choice: refuse rather than keep per-package modes

The issue offers two fixes: keep each package in its current mode, or refuse as ambiguous. This PR refuses, for three reasons:

  • The CLI already does exactly this for agent + vendored state.
  • Keeping per-package modes would mean running the hosted and vendored engines for different packages in one run. That needs new update semantics for a hosted-pinned package with a newer patch, and new dry-run/JSON reporting. It's a much larger change.
  • Refusing is the conservative choice: nothing gets converted without an explicit --mode.

/code-review high raised the alternative. Its other findings that I left as they are:

  • A bare get <pkg> is refused even when <pkg> isn't hosted-pinned. Agent + vendored state already behaves the same way. Fixing it would mean resolving the package before the mode is chosen. The user can pass --mode.
  • With PATH arguments, a mixed --cwd is refused before each directory's mode is worked out. That's existing behavior for agent + vendored too. The cwd mode also decides whether PATHs are project dirs or agent globs, so the pre-check can't simply be skipped.
  • One extra lockfile walk on bare runs in vendored projects. It happens only when a ledger exists. Reusing the scan context's discovery would mean threading it through the vendored engine, which is out of scope here.
  • A test fixture is duplicated with scan/mod.rs. It's a small test-only helper in a different module.

Test evidence

Red on main (before the fix commit), green with it:

  • cargo test -p socket-patch-cli --all-features --lib -- commands::tests: hosted_pins_beside_a_ledger_are_ambiguous and a_hosted_pin_over_a_ledger_entry_is_ambiguous FAILED before the fix (returned Ok(Vendored)), and all 9 pass after. a_lockless_hosted_pin_beside_a_ledger_is_ambiguous covers the nuget lockless pin. a_gemfile_only_hosted_pin_beside_a_ledger_is_ambiguous covers the Gemfile-only gem pin: it fails with the Gemfile read commented out and passes with it.
  • cargo test -p socket-patch-cli --all-features --test scan_get_mode_from_project_state: bare_scan_and_get_refuse_a_project_with_vendored_and_hosted_state FAILED before the fix (left: 0, right: 2: the bare scan exited 0) and passes after. It checks bare scan and bare get: exit 2, mode_ambiguous, lock, ledger, manifest and .npmrc byte-identical, hosted pin kept. It also checks that an explicit --mode vendored still succeeds.

Local runs:

  • cargo fmt --all -- --check: OK
  • cargo clippy --workspace --all-features -- -D warnings: OK
  • cargo test -p socket-patch-cli --all-features --lib plus in_process_get_modes, in_process_vendor_{bun,npm_v1,pnpm,pypi}_takeover, mode_migration_{bun,cargo,npm,pypi,vlt} and scan_get_mode_from_project_state: 1,199 passed, 0 failed. The full cargo test --workspace run hit the container's disk limit while building every test binary, so CI covers the rest.

Per-issue checklist

🤖 Generated with Claude Code

https://claude.ai/code/session_01Pg9M6ADMS1SLLmpFkvmA5r

Generated by Claude Code

@claude
Start fix for #1437
8f15795 
Assisted-by: Claude Code:claude-opus-5-5
Closed
@claude
Refuse bare scan/get on vendored + hosted projects
1601aa1 
A bare `scan` or `get` (no --mode) picks its mode from the project's
state, but only the vendor ledger and the manifest counted. Hosted
mode keeps no ledger, so a project holding vendored packages beside
hosted pins was classed as vendored, and the run silently took every
hosted pin over to vendored: exit 0, no warning. That undid a partial
`--mode hosted` takeover and per-package `get --mode` choices.

When the vendor ledger is non-empty, the mode decision now also reads
the lockfiles' hosted pins. Vendored + hosted state is a usage error
(exit 2, `mode_ambiguous`) that names the pinned packages and asks for
--mode, the same as agent + vendored state already is. Nothing is
written. Projects with only one kind of state are unaffected, and
hosted-only projects do not pay for the extra lockfile walk.

Fixes #1437

Assisted-by: Claude Code:claude-opus-5-5
@claude
Count lockless hosted pins as hosted state
155f868 
A cargo or nuget package pinned to Socket's patch registry with no
lockfile entry yet is still hosted, but the bare-run mode check only
looked at lockfile pins. A vendored project beside such a pin could
still be silently converted. Count those pins too, and tighten the
end-to-end test so an explicit --mode must succeed, not just avoid
exit 2.

Refs #1437

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 12, 2026 17:41
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot]
cursor Bot reviewed Oct 12, 2026
View reviewed changes

@cursor cursor Bot left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-cli/src/commands/mod.rs
@claude
Count Gemfile-only gem pins as hosted state
0a72a49 
On a Gemfile.lock with no CHECKSUMS section, hosted mode pins a gem
in the Gemfile only and leaves the lock for the next bundle install.
That pin has no lockfile entry yet, so the bare-run mode check missed
it, and a vendored project beside it could still be converted. Read
those Gemfile pins too.

Refs #1437

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot]
cursor Bot reviewed Oct 12, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 0a72a49. Configure here.

Open
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 12, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

Ready for review (burn-down agent)

  • Head: 0a72a494
  • CI: 35/35 green, 19 path-skipped; no main-wide failures.
  • Mergeable: yes, no conflicts with main.
  • Bugbot: reviewed 0a72a49, no new issues; all review threads resolved.
  • Reviewer focus: the change to mode_from_project_state, which now counts hosted pins so a mixed vendored + hosted project isn't classed vendored.

Generated by Claude Code

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. A bare scan/get (no --mode) on a project that has a vendor ledger and hosted pins in its dependency files now exits 2 with mode_ambiguous and writes nothing. Before, it silently kept vendored mode and took every hosted pin over to vendored (#1437). This matches how agent + vendored state is already handled.

Risk: low–medium. The change is one match arm in mode_from_project_state, and it only runs when a ledger exists. Hosted-only and vendored-only projects behave as before. The only behavior change is a new exit 2 for mixed projects, and both the contract and the migration doc describe it.

Look here

  • commands/mod.rs:117: the new (ledger, no agent) arm. It collects hosted state from HostedPin::recorded, lockless cargo/nuget pins and Gemfile-only gem pins, and names up to 3 of them in the error.
  • commands/mod.rs:532 to :581: unit tests for npm, hosted-over-ledger, nuget lockless and Gemfile-only cases.
  • tests/scan_get_mode_from_project_state.rs:496:`` end-to-end test. Bare scan and `get` both exit 2, files stay byte-identical, the hosted pin is kept, and an explicit `--mode vendored` still succeeds.
  • CLI_CONTRACT.md:178: the contract text.

Verified

  • Read the full diff, both call sites (scan/mod.rs mode_inferred and get.rs None => arm), which map the Err to mode_ambiguous, and discover_wiring, which reuses the run's --patch-server-url origins.
  • The PR description reports these tests failing on main before the fix commit and passing after it.
  • CHANGELOG.md is untouched. There is no debug code and no unrelated edits.
  • CI on 0a72a49: 35 passed, 19 skipped, 0 failed (ci-ok and clippy green). Bugbot is clean on the head commit. The one review thread is resolved.

Changes I made. None.

Open questions (not blocking)

  • A hosted→vendored takeover can refuse some pins (the pnpm lock-text and gem takeover gates). After that, the project keeps a ledger and the refused hosted pins, so every later bare scan/get in it exits 2 until --mode is passed. This follows from "switching modes is always explicit", but CI scripts that run a bare scan in such repos will start failing. Is that the intended UX, or should refused takeover pins be left out of the check?
  • --ecosystems doesn't narrow the check, so a hosted gem pin makes a bare scan --ecosystems npm ambiguous too. That's conservative, and agent + vendored works the same way.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

Generated by Claude Code

Merged via the queue into main with commit d1a1ca7 Oct 12, 2026
54 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-bare-mode-mixed-hosted-vendored branch October 12, 2026 20:25
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments
@Tanmay182003 Tanmay Singla (Tanmay182003) Tanmay182003 approved these changes

Assignees

No one assigned

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

3 participants

@mikolalysenko @Tanmay182003 @claude